Skip to content

feat(objectql)!: hook 的 condition 求不出值时 fail loud —— 抛错并中断该次操作 (#4775) - #4861

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-4775-hook-condition-fail-loud
Aug 3, 2026
Merged

feat(objectql)!: hook 的 condition 求不出值时 fail loud —— 抛错并中断该次操作 (#4775)#4861
os-zhuang merged 4 commits into
mainfrom
claude/issue-4775-hook-condition-fail-loud

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #4775

按维护者拍板的**方案 B(全局 fail loud)**实现,并同批实现 #4800B1(predicate 批量写的专门诊断)。

做了什么

hook 的 condition 求不出值时,原先是 logger.warn + return false —— 把「表达式说不」和「平台算不出表达式说什么」压成同一个结果。这两件事对不同 hook 是相反方向的风险:before* 守卫被吞成 false放行一次本该被拦的写入,after* 审计被吞成 false漏记一条没人会去找的记录。

现在:求不出值 → 抛 HookConditionError,中断该次操作,错误点名 hook 与出问题的 key。before*after* 同一个方向,这是明知并接受的代价 —— 一条规则一个答案,平台不多长一条「要看 event 类别才知道失败方向」的隐性规则。

编译不过的 condition 也一并阻断。 它原先的处置(condition ignored)是这次吞咽里更糟的一半:整个门被删掉了,于是声明来拦截的守卫放行了每一次写入、审计 hook 在每一次写入上都触发。改为在调用时报错而非 bind 时,这样一条坏 hook 不会把没人写入的 app 的启动卡死。

没有改变的部分

  • condition 求值为 FALSE 仍然只是跳过,写入照常成功。只有「求不出值」是新行为。
  • onError 未被接入,这是刻意的。 它管辖的是 handler 抛错;condition 求值发生在 handler 之前。接进去会让 onError: 'log' 把本次要消灭的静默跳过原地复活,并长出第三套语义。retryPolicy / async 同理留在门外。

错误文案与 #4649 同源

新增 cel-fault.ts,由 hook condition 与 validation 谓词(rule-validator.ts)共用。两个因同一原因拒写的求值器不该用两种方言描述它 —— 与当初把 materializeDeclaredFields 抽出来共用是同一个理由。rule-validator.ts 这边是纯重构,对外错误形状不变。

B1:predicate 批量写的专门诊断 (#4800)

批量写匹配 N 行、hook 只触发一次,所以 previous 不绑定、record 只是裸 payload。fail loud 不开例外,但文案是诊断而不是谜语:点名 hook、说明「这是 predicate 批量更新,没有单一前置记录」、给出可行的路(改写掉 previous,或按 id 单记录写入)。

批量写上的未声明 key 仍然走普通的拼写错误文案 —— 那个确实是拼错,说成批量问题会把作者引去修一个拼写正确的字段。

🔴 硬防线:record-change flow trigger 这条出路已核实,并被否决

文案里没有写「改用 record-change flow trigger」。核实结论(实测,非推断):

  • packages/triggers/trigger-record-change/src/record-change-trigger.ts:252 通过 engine.registerHook 订阅的正是这同一批生命周期 hook;
  • 它读 ctx.previous ?? ctx.__previous(第 291-293 行);
  • 引擎只在单 id 分支赋值 hookContext.previous(engine.ts:4888),批量分支 priorRecord 始终为 null;
  • plugin-audit 的 __previous 兜底也在批量上直接返回(audit-writers.ts:425,if (!id) return;)。

所以 flow trigger 在批量写上拿到的是同样未绑定的 previous —— 它不是出路。在一个修 declared ≠ enforced 的 PR 里,于错误信息中指一条自己没验证过的路,就是当场再造一个同类缺陷。flow trigger 的批量语义已在报告中交回 PM 另立单。

落地前的全仓扫描(含 examples/)

仓内真正的 hook condition 只有两条,都在 showcase,且都是对的:

hook condition 结论
showcase_audit_task_completion previous.done != true && record.done == true 单记录写正常;这正是 #4784 教的写法
showcase_warn_over_budget record.spent != null && record.budget != null && record.spent > record.budget 已按 #4763/#4649 的教训用 != null 而非 has() 守卫

showcase 三个 update_record 节点全部按 id 过滤,roll-up(recomputeSummaries)也是按 id 单记录更新,仓内没有任何一条已编写的批量写路径会踩到 B1 那一格。

⚠️ 注意 examples/app-showcase/src/automation/flows/index.ts 里那些 status == "done" && previous.status != "done"flow / automation 的扁平作用域,与 hook conditionrecord 作用域是两个面,不受本改动影响。

扫描当场揪出一条从没真正生效过的条件

hook-metrics.test.ts 里的 condition: 'name == "skipme"'裸标识符。hook condition 是 record 作用域的,裸标识符解析不到任何东西,该表达式每次调用都在 fault —— 那条测试断言的「skip」来自旧的吞咽(fault → warn → false),而不是来自一个回答了「否」的条件。这正是本 issue 预期会揪出来的东西。已改为 record.name == "skipme",同样的意图,但真的被求值了。

hook-binder.test.ts 里那条 pin 则显式接受两种结果(「要么编译期忽略(handler 跑)、要么求值 false(跳过)…只断言没崩」),那份宽容本身就是缺陷;已改为断言拒绝、uncompilable 原因与 handler 未运行。

破坏性 & changeset

已加 .changeset/hook-condition-fail-loud.md,标 major,写清「存量靠静默跳过苟着的 hook 会开始让写入失败」,并附迁移清单(拼错/退役字段、未加保护的 null 比较、批量上的 previous、裸标识符)。

验证

命令 结果
pnpm --filter @objectstack/objectql test 110 files / 1741 tests passed
pnpm --filter @objectstack/objectql typecheck tsc --noEmit 无输出
eslint(全部改动文件) 0 errors
@objectstack/rest 38 / 573 passed
@objectstack/runtime 80 / 1092 passed
@objectstack/plugin-approvals 18 / 402 passed(含新落地的 record-lock-multi-update)
@objectstack/plugin-sharing 11 / 243 passed
@objectstack/trigger-record-change 4 / 51 passed
turbo run build(全仓,排除 docs) 71 tasks successful,含 example-showcase
@objectstack/spec check:generated 8 / 8 up to date

新增 hook-condition-fail-loud.test.ts(504 行)覆盖:求不出值即拒绝且点名 hook 与 key、与 #4649 共用同一句文案、before*/after* 同向且无 event 分档、FALSE 仍只是跳过(改动的爆炸半径,最容易误伤的一条)、onError/retryPolicy 不参与、B1 四格诊断、以及编译不过的 condition。

归口约束

未改动 skills/**packages/lintcontent/docs/**packages/spec/**。改动仅落在 .changeset/packages/objectql/**skills/objectstack-formula/SKILL.md §5 该补的那一行已写进给 PM 的报告,由 #4814 在对侧车道补上。

🤖 Generated with Claude Code


Generated by Claude Code

claude added 4 commits August 3, 2026 09:57
…4775)

A declarative hook whose `condition` could not be evaluated emitted a
`logger.warn` and returned `false`, collapsing "the condition said no" and
"the platform could not work out what the condition says" into one outcome —
which carries opposite risks per hook kind: a `before*` guard swallowed into
`false` lets through a write it was declared to stop, and an `after*` audit
drops a row nobody will go looking for.

Resolve it the way #4649 already resolved it for validation predicates: reject
loudly, naming the hook and the key that would not resolve. `before*` and
`after*` take the same direction, knowingly. A condition that never compiled
aborts too, reported at invocation so one broken hook cannot wedge boot.

The fault wording is shared with `rule-validator.ts` through a new
`cel-fault.ts`, so two evaluators that reject a write for the same reason
cannot describe it in two dialects.

Predicate (`multi: true`) bulk writes get a dedicated diagnosis (#4800/B1)
rather than a bare `No such key: previous`: the hook is named, the batch is
explained, and the route that works is given. It deliberately does not name a
record-change flow trigger as a way out — that trigger binds these same
lifecycle hooks and receives the same unbound `previous` on a bulk write
(verified against the engine and `trigger-record-change`).

`onError`, `retryPolicy` and `async` are untouched and stay outside this gate:
condition evaluation happens before the handler exists to fail, and routing it
through `onError` would let `onError: 'log'` resurrect the silent skip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
…tes (#4775)

`hook-binder.test.ts` pinned the OLD uncompilable behaviour and explicitly
accepted either outcome ("ignored at compile time (handler runs) or evaluated
false (skipped) … just assert we didn't crash"). That latitude was the defect:
"condition ignored" deleted the gate, so a hook declared to run conditionally
ran on every write. It now asserts the rejection, the `uncompilable` reason and
that the handler never ran.

`hook-metrics.test.ts` used a BARE `name == "skipme"` condition. Hook
conditions are `record`-scoped, so the bare identifier resolved to nothing and
the expression faulted on every call — the "skip" the test asserted came from
the old swallow (fault → warn → false), not from a condition that answered NO.
This is precisely the never-actually-enforced condition #4775 was expected to
flush out. Rewritten as `record.name == "skipme"`, which expresses the same
intent and is actually evaluated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 3, 2026 10:34am

Request Review

@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation tests tooling labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql.

13 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx (via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx (via @objectstack/objectql)
  • content/docs/deployment/vercel.mdx (via @objectstack/objectql)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/objectql)
  • content/docs/kernel/services.mdx (via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx (via @objectstack/objectql)
  • content/docs/plugins/index.mdx (via @objectstack/objectql)
  • content/docs/plugins/packages.mdx (via @objectstack/objectql)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx (via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx (via @objectstack/objectql)
  • content/docs/releases/implementation-status.mdx (via @objectstack/objectql)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Copy link
Copy Markdown
Contributor Author

范围外发现已立单:#4862

正文里「flow trigger 的批量语义已在报告中交回 PM 另立单」那一条,已按 PD #10 直接立单为 #4862(未指派),以免只存在于报告里。

核实 B1 出路时顺带测出的、比预期更重的一格:predicate 批量写上 record-change flow trigger 不仅 previous 不绑定,record 也只是裸 payload —— 引擎批量分支 driver.updateMany 返回的是受影响行数(number),buildContextafter && typeof after === 'object' 因此为 false,record 退化成本次写入的 payload,而不是任何一行的状态。

要紧的是这一格正是官方文档与 showcase 在教的写法落点:showcase 有 10 条 flow 的起始条件是 status == "done" && previous.status != "done" 这个形状。对一次批量置 done,它们要么不触发、要么按一条并不存在的「记录」触发一次,且完全静默。

对照本 PR 值得记一笔:hook condition 在这一格从此响亮失败(B1 专门诊断),而 flow trigger 那一格依然静默 —— 同一个平台对同一个物理限制给了两种相反的处理。#4862 建议与 #4800 一起定方向。

本 PR 不处理它,也不因它扩大范围。


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 3, 2026 10:38
@os-zhuang
os-zhuang enabled auto-merge August 3, 2026 10:38

Copy link
Copy Markdown
Contributor Author

复核通过 —— ACCEPT,已标 ready 并送合并队列(发版关键项)

本单同样是接手一个被宿主上下文压缩机械中断的 agent 的未提交成果。接手方没有照单全收,核出并修掉三处缺陷 —— 其中一处的性质值得单独讲。

那条修在自家测试里的缺陷,正是本 issue 存在的理由

hook-metrics.test.ts 原本写着:

condition: 'name == "skipme"'      // 裸标识符,每次调用都在 fault

它断言的「skip」不是条件回答了「否」,而是旧的静默吞咽。也就是说:一条一直在通过的测试,证明的是一个从来没真正求值过的条件。改成 record.name == "skipme" 后才是真的在测它声称要测的东西。

这与 #4649 用同一招揪出两条我们自己的示例规则、与今早 #4837 / #4839 的形态完全同源。本 PR 上线前,这一招在本仓已经四次找到"声明了但从未生效"的东西 —— 而这次是在测试自己身上。 派发时我说「预期它会揪出若干条从没真正生效过的条件,那是该被发现的东西,不是要绕过的噪音」,这就是那个东西。

另两处也修得对:hook-binder.test.ts 那条 pin 显式接受两种结果(新代码下必然失败,且它本来就是一条不表态的测试),已改为断言拒绝与 uncompilable;以及残留的未使用解构。

硬防线守住了 —— 而且答案比我要求的更强

我的派发文里写了一条红线:

错误文案里写「改用 record-change flow trigger」这条出路之前,必须先核实它在批量写上真的按行拿到 previous。不允许在一个修 declared ≠ enforced 的 PR 里,于错误信息中再造一个同类缺陷。

dev 核实了,结论是不通,于是错误文案里写的是:

A record-change flow trigger is NOT a way around this ...

没有把一条自己没验证过的出路写进错误信息 —— 反而主动把它标成「此路不通」。 这正是这条红线要保护的东西。

并且把不通的原因查清了,立为 #4862:predicate 批量写上 flow trigger 不但 previous 不绑定,连 record 都只是裸 payload —— 因为引擎批量分支 driver.updateMany 返回的是行数不是行,buildContextafter && typeof after === 'object' 因此为 false。

#4862 里最要命的一条:showcase 有 10 条 flow 起始条件是 status == "done" && previous.status != "done" 形状 —— 正是官方文档教的写法。批量置 done 时,它们要么不触发、要么按一条不存在的「记录」触发一次,且完全静默

于是本 PR 之后出现一个需要收口的不对称:同一格上,hook condition 从此响亮失败,flow trigger 依然静默。建议 #4862#4800 一起定方向。

关键假设是实测的,不是推断的

B1 的分支靠区分 Unknown variableNo such key 工作 —— 若 formula 的文案不符,那条分支就是死代码。dev 用真实 @objectstack/formula dist 探针把四种 fault 文案全测了一遍:

场景 实测文案
unbound previous type: Unknown variable: previous
未声明 key runtime: No such key: nope
null 比较 runtime: no such overload: dyn<null> < dyn<double>
语法错 parse: Unexpected token: EQ

一个「修 declared ≠ enforced」的 PR,自己不能靠假设别人的输出形状办事。 这一步做对了。

一处扩展我认可

「编译不过的 condition 同样阻断」—— issue 没明写,但 dev 的理由站得住:旧的 condition ignored更糟的一半,它把门整个删掉了。同时把报错时机放在调用时而非启动时,避免卡住启动。方向与方案 B 一致,不是夹带。

约束核对

我自己补做的一项核验:examples/ 扫描

派发第 5 条要求「落地前扫仓内(含 examples/)hook condition,把 B 会打挂的先修掉」,报告未明确回应,我自行扫了 origin/main。全仓 examples/ 下只有 app-showcase/src/data/hooks/index.ts 定义了 hook condition,两条:

condition: "previous.done != true && record.done == true"
condition: "record.spent != null && record.budget != null && record.spent > record.budget"

第二条只用 record 且显式判空,安全。第一条引用 previous,在 predicate 批量写上会被本 PR 打挂 —— 但不动它是正确的:

  1. 单记录写入路径下它完全合法(bug(objectql/docs): hook condition 的 CEL 作用域只绑定 record —— 文档教的 previous.x / ctx.record 根本不存在,过渡型条件写不出来 #4784 已让 previous 进作用域,该文件注释明写「compares against previous on purpose (bug(objectql/docs): hook condition 的 CEL 作用域只绑定 record —— 文档教的 previous.x / ctx.record 根本不存在,过渡型条件写不出来 #4784)」);
  2. 改写它以「兼容批量」等于撤销刚落地的 bug(objectql/docs): hook condition 的 CEL 作用域只绑定 record —— 文档教的 previous.x / ctx.record 根本不存在,过渡型条件写不出来 #4784 决定;
  3. 批量那一格失败正是 B1 拍板的预期行为,且现在会拿到点名 hook 的专门诊断,不是 No such key: previous

所以这一格是「已扫、经判断不改」,不是「漏扫」。 记录在此以免日后被当成缺口。它同时说明 #4862 的紧迫性:showcase 里 hook 那一格响亮、flow 那一格静默,两种体验会同时出现在同一个示例应用里。


⚠️ 若合并队列把本 PR 踢出,先确认 #4856(spec vitest testTimeout 止血)是否已合:未合则可能仍是 #4796 那条 flaky,原样重投;已合还红则不是它,必须重新诊断。


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hook 的 condition 求不出值时:全局 fail loud —— 抛错并中断该次操作(方案 B 已拍板;Blocked-by #4770)

2 participants